You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Adversarial Self-Review

Issues Found and Addressed

1. Misleading maxResults parameter (FIXED in a8bfc06)
The SearchController accepted a maxResults query parameter that was forwarded to the service but never actually used -- the service uses internal MaxBoardResults=10 / MaxCardResults=20 constants. Removed the parameter to avoid a misleading API contract.

Issues Reviewed and Accepted

2. XSS in search results -- NOT an issue
All search result text (board names, card titles, descriptions) is rendered via Vue's {{ }} interpolation which auto-escapes HTML. No v-html usage. Safe.

3. Board search is in-memory filtering
GetReadableByUserIdAsync loads all readable boards for the user, then the service filters in memory with StringComparison.OrdinalIgnoreCase. This matches existing patterns in WorkspaceService. For users with hundreds of boards this could be optimized later with a server-side query, but is fine for current scale.

4. Card search case sensitivity
The SearchAcrossBoardsAsync uses EF Core .Contains() which translates to SQLite LIKE -- case-insensitive for ASCII characters. Matches the existing CardRepository.SearchAsync pattern. Acceptable.

5. Search debouncing
Frontend uses 200ms debounce in useGlobalSearch, with abort-on-supersede for in-flight requests. This prevents excessive API calls during fast typing.

6. _cardId parameter is unused in handleNavigateToCard
Currently navigates to the board containing the card. A future enhancement could scroll/highlight the specific card. Prefixed with _ to signal intentional non-use.

7. No backend unit tests for SearchService
The service is thin (delegates to existing repository methods that are already tested). The controller follows the established pattern used by all other controllers. Frontend coverage is thorough with 19 new tests.

Accessibility

• ARIA roles maintained: dialog, combobox, listbox, option
• aria-activedescendant tracks the selected item
• aria-selected on options
• Keyboard navigation: arrows, enter, escape all work across all result types

Performance

• 200ms debounce prevents excessive API calls
• Minimum 2-character query threshold before hitting the backend
• Server-side Take() caps card results at 20

Adversarial Code Review -- PR #603 (Global Search Launcher)

Overall this is a well-structured feature. The authorization model is correct (scoped via GetReadableByUserIdAsync), the controller is [Authorize]d, tests are decent. However, I found several issues ranging from a real bug to performance concerns.

BUG -- Abort controller is created but never wired up (Severity: Medium)

File: frontend/taskdeck-web/src/composables/useGlobalSearch.ts, lines ~40-50

The composable creates an AbortController and calls .abort() on supersede, but never passes the signal to searchApi.search(). The API call signature is:

const result = await searchApi.search(searchQuery.trim())

And searchApi.search does not accept a signal parameter. Meanwhile searchApi uses Axios, which supports { signal } in config. The abort-on-supersede logic is therefore dead code -- previous HTTP requests will complete and their results will overwrite the current results because the finally block sets loading.value = false unconditionally.

Repro: Type "tes" (debounce fires request A), then quickly append "ting" (debounce fires request B). If request A resolves after request B, the user sees stale results for "tes" instead of "testing".

Fix: Pass { signal: abortController.signal } through to the Axios call and accept it in searchApi.search.

PERFORMANCE -- GetReadableByUserIdAsync loads ALL boards into memory (Severity: Medium)

File: backend/src/Taskdeck.Application/Services/SearchService.cs, lines ~22-28

var readableBoards = (await _unitOfWork.Boards.GetReadableByUserIdAsync(
    userId, includeArchived: false, cancellationToken)).ToList();

This materializes every board the user can access, then filters in-memory with string.Contains. For a user with hundreds of boards, this is wasteful. The card search is properly server-side via SearchAcrossBoardsAsync, but the board search does a full in-memory scan.

Consider adding a SearchReadableByUserIdAsync method that pushes the text filter into the SQL query, or at minimum add a Take() limit to the readable boards query.

PERFORMANCE -- No query length cap (Severity: Low-Medium)

File: backend/src/Taskdeck.Application/Services/SearchService.cs

The minimum query length is 2 characters, but there is no maximum. A client can send a multi-KB query string. While SQLite's LIKE/INSTR is generally tolerant, extremely long search strings:

• Waste bandwidth
• Could cause unexpected behavior with EF Core's Contains translation
• Could be used for low-effort DoS

Recommend capping at ~200 characters.

PERFORMANCE -- SearchAcrossBoardsAsync with large IN clause (Severity: Low)

File: backend/src/Taskdeck.Infrastructure/Repositories/CardRepository.cs, new method

.Where(c => materializedBoardIds.Contains(c.BoardId))

EF Core translates this to WHERE BoardId IN (...). For a user with access to hundreds of boards, this generates a very large IN clause. SQLite handles this reasonably, but it's worth noting as a scalability concern. Consider batching or using a temp table approach if board counts grow.

CORRECTNESS -- Case sensitivity inconsistency between board and card search (Severity: Low)

Board search (in SearchService.cs) uses StringComparison.OrdinalIgnoreCase:

b.Name.Contains(trimmedQuery, StringComparison.OrdinalIgnoreCase)

Card search (in CardRepository.SearchAcrossBoardsAsync) uses EF Core's Contains without a comparison:

.Where(c => c.Title.Contains(searchText) || c.Description.Contains(searchText))

SQLite's default LIKE is case-insensitive for ASCII but case-sensitive for non-ASCII. The board search (in-memory) is consistently case-insensitive. The card search depends on SQLite collation. This is consistent with the existing CardRepository.SearchAsync, so it's not a regression, but it's an inconsistency in this feature's own behavior.

SECURITY -- No rate limiting on search endpoint (Severity: Low)

File: backend/src/Taskdeck.Api/Controllers/SearchController.cs

The search endpoint has no rate limiting. Combined with the fact that it loads all readable boards + does a card search on every request, this is a potential DoS vector. An authenticated attacker could hammer the endpoint. The frontend debounces at 200ms, but nothing stops direct API abuse.

TEST GAP -- No backend tests for SearchService (Severity: Medium)

There are no unit tests for SearchService in the diff. The frontend composable and component are well-tested, but the backend service layer -- which contains the authorization logic and result mapping -- has zero test coverage. Key untested scenarios:

• Empty userId returns validation error
• Short query returns empty results
• User can only see boards they own or have access to (authorization)
• Cards from inaccessible boards are excluded
• maxResults parameter is respected
• Board/column names resolve correctly (the c.Board?.Name ?? "Unknown" fallback)

TEST GAP -- error state never surfaced to user (Severity: Low)

File: frontend/taskdeck-web/src/composables/useGlobalSearch.ts

The composable exposes error but it is never used in ShellCommandPalette.vue. If a search fails, the user sees "No results found." with no indication of failure. The error ref is returned from useGlobalSearch but not destructured or displayed in the palette.

MINOR -- Redundant maxResults parameter (Severity: Nitpick)

ISearchService.SearchAsync accepts maxResults = 20 but SearchService ignores it completely, using hardcoded MaxBoardResults = 10 and MaxCardResults = 20 instead. The controller doesn't expose it either. Either remove the parameter or wire it through.

Summary

The abort controller bug is the most actionable -- it's a real correctness issue that will manifest as stale search results during fast typing. The missing backend tests should also be addressed before merge.